update libnest to v8 #1226
update libnest to v8 #1226
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughMajor authentication system overhaul with CASL-based authorization and JWT strategies. Introduces new auth module, ability factory, JWT guards, and decorators. Migrates types (AppAbility, RouteAccess) from external package to local implementations. Refactors Prisma integration with new factory and MongoDB support. Removes instrument library DTOs and several form definitions. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthController
participant AuthService
participant AbilityFactory
participant JwtService
participant UsersService
Client->>AuthController: POST /auth/login (credentials)
AuthController->>AuthService: login(credentials)
AuthService->>UsersService: findByUsername(username, includeHashedPassword)
UsersService-->>AuthService: user + hashedPassword
AuthService->>AuthService: verifyPassword(credentials.password, user.hashedPassword)
AuthService->>AbilityFactory: createForPayload(tokenPayload)
AbilityFactory-->>AuthService: AppAbility
AuthService->>JwtService: sign({ ...tokenPayload, permissions })
JwtService-->>AuthService: accessToken
AuthService-->>AuthController: { accessToken }
AuthController-->>Client: 200 { accessToken }
Client->>+AuthController: GET /resource (Bearer token)
AuthController->>JwtAuthGuard: canActivate(context)
JwtAuthGuard->>JwtStrategy: validate(payload)
JwtStrategy->>AbilityFactory: createForPermissions(payload.permissions)
AbilityFactory-->>JwtStrategy: AppAbility
JwtStrategy-->>JwtAuthGuard: { ...payload, ability }
JwtAuthGuard->>JwtAuthGuard: check route access + ability permissions
JwtAuthGuard-->>AuthController: true (authorized) / false (denied)
alt Authorized
AuthController-->>Client: 200 resource
else Unauthorized
AuthController-->>Client: 403 Forbidden
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (52)
apps/api/libnest.config.ts(1 hunks)apps/api/package.json(3 hunks)apps/api/src/assignments/assignments.controller.ts(1 hunks)apps/api/src/assignments/assignments.service.ts(1 hunks)apps/api/src/auth/__tests__/ability.utils.test.ts(1 hunks)apps/api/src/auth/ability.factory.ts(1 hunks)apps/api/src/auth/ability.utils.ts(1 hunks)apps/api/src/auth/auth.controller.ts(1 hunks)apps/api/src/auth/auth.module.ts(1 hunks)apps/api/src/auth/auth.service.ts(1 hunks)apps/api/src/auth/auth.types.ts(1 hunks)apps/api/src/auth/guards/__tests__/jwt-auth.guard.spec.ts(1 hunks)apps/api/src/auth/guards/jwt-auth.guard.ts(1 hunks)apps/api/src/auth/strategies/jwt.strategy.ts(1 hunks)apps/api/src/core/decorators/route-access.decorator.ts(1 hunks)apps/api/src/core/prisma.ts(1 hunks)apps/api/src/core/schemas/env.schema.ts(1 hunks)apps/api/src/core/schemas/mongo-stats.schema.ts(1 hunks)apps/api/src/core/types.ts(1 hunks)apps/api/src/demo/demo.service.ts(5 hunks)apps/api/src/gateway/gateway.controller.ts(1 hunks)apps/api/src/groups/groups.controller.ts(1 hunks)apps/api/src/groups/groups.service.ts(1 hunks)apps/api/src/instrument-records/dto/create-instrument-record.dto.ts(0 hunks)apps/api/src/instrument-records/dto/update-instrument-record.dto.ts(0 hunks)apps/api/src/instrument-records/dto/upload-instrument-record.dto.ts(0 hunks)apps/api/src/instrument-records/instrument-records.controller.ts(3 hunks)apps/api/src/instrument-records/instrument-records.service.ts(2 hunks)apps/api/src/instruments/instruments.controller.ts(1 hunks)apps/api/src/instruments/instruments.service.ts(2 hunks)apps/api/src/main.ts(2 hunks)apps/api/src/sessions/sessions.controller.ts(1 hunks)apps/api/src/sessions/sessions.service.ts(1 hunks)apps/api/src/setup/setup.controller.ts(1 hunks)apps/api/src/setup/setup.service.ts(4 hunks)apps/api/src/subjects/__tests__/subjects.service.spec.ts(1 hunks)apps/api/src/subjects/subjects.controller.ts(1 hunks)apps/api/src/subjects/subjects.service.ts(1 hunks)apps/api/src/summary/summary.controller.ts(1 hunks)apps/api/src/typings/global.d.ts(1 hunks)apps/api/src/users/users.controller.ts(1 hunks)apps/api/src/users/users.service.ts(2 hunks)apps/api/src/vendor/configured.auth.module.ts(0 hunks)apps/web/src/components/DemoBanner/DemoBanner.tsx(1 hunks)apps/web/src/components/LoginForm/LoginForm.tsx(1 hunks)apps/web/src/routes/auth/login.tsx(3 hunks)package.json(1 hunks)packages/schemas/src/auth/auth.ts(1 hunks)packages/schemas/src/instrument-records/instrument-records.ts(2 hunks)testing/e2e/src/global/global.setup.spec.ts(1 hunks)testing/e2e/src/helpers/types.ts(1 hunks)testing/k6/src/index.ts(2 hunks)
💤 Files with no reviewable changes (4)
- apps/api/src/instrument-records/dto/upload-instrument-record.dto.ts
- apps/api/src/vendor/configured.auth.module.ts
- apps/api/src/instrument-records/dto/update-instrument-record.dto.ts
- apps/api/src/instrument-records/dto/create-instrument-record.dto.ts
🧰 Additional context used
🧬 Code graph analysis (24)
apps/api/src/auth/strategies/jwt.strategy.ts (4)
apps/api/src/auth/ability.factory.ts (1)
Injectable(11-63)apps/api/src/auth/auth.service.ts (1)
Injectable(11-60)apps/api/src/auth/guards/jwt-auth.guard.ts (1)
Injectable(12-56)packages/schemas/src/auth/auth.ts (1)
TokenPayload(17-25)
apps/api/src/auth/auth.service.ts (4)
apps/api/src/auth/ability.factory.ts (1)
Injectable(11-63)apps/api/src/auth/strategies/jwt.strategy.ts (1)
Injectable(11-28)apps/api/src/users/users.service.ts (1)
Injectable(13-167)packages/schemas/src/auth/auth.ts (3)
$LoginCredentials(11-11)$LoginCredentials(12-15)TokenPayload(17-25)
apps/api/src/subjects/__tests__/subjects.service.spec.ts (1)
apps/api/src/core/prisma.ts (1)
RuntimePrismaClient(67-69)
apps/api/src/core/prisma.ts (5)
apps/api/src/sessions/sessions.service.ts (1)
Injectable(15-121)apps/api/src/demo/demo.service.ts (1)
Injectable(35-173)apps/api/src/setup/setup.service.ts (1)
Injectable(16-95)apps/api/src/subjects/subjects.service.ts (1)
Injectable(12-146)apps/api/libnest.config.ts (1)
PrismaClient(18-18)
apps/api/src/auth/auth.controller.ts (3)
apps/api/src/assignments/assignments.controller.ts (1)
Controller(13-37)apps/api/src/core/decorators/route-access.decorator.ts (1)
RouteAccess(18-20)packages/schemas/src/auth/auth.ts (2)
$LoginCredentials(11-11)$LoginCredentials(12-15)
apps/web/src/routes/auth/login.tsx (1)
packages/schemas/src/auth/auth.ts (2)
$LoginCredentials(11-11)$LoginCredentials(12-15)
apps/web/src/components/LoginForm/LoginForm.tsx (1)
packages/schemas/src/auth/auth.ts (2)
$LoginCredentials(11-11)$LoginCredentials(12-15)
apps/api/src/auth/ability.factory.ts (4)
apps/api/src/auth/auth.service.ts (1)
Injectable(11-60)apps/api/src/auth/strategies/jwt.strategy.ts (1)
Injectable(11-28)packages/schemas/src/auth/auth.ts (1)
TokenPayload(17-25)apps/api/src/auth/ability.utils.ts (2)
detectAppSubject(13-18)createAppAbility(20-24)
apps/api/libnest.config.ts (2)
apps/api/src/core/schemas/env.schema.ts (2)
$Env(5-5)$Env(6-35)apps/api/src/core/prisma.ts (1)
RuntimePrismaClient(67-69)
testing/e2e/src/global/global.setup.spec.ts (4)
testing/e2e/src/helpers/fixtures.ts (1)
test(11-17)testing/e2e/src/pages/__root.page.ts (1)
expect(12-14)testing/e2e/src/helpers/data.ts (1)
initAppOptions(3-14)packages/schemas/src/auth/auth.ts (2)
$LoginCredentials(11-11)$LoginCredentials(12-15)
apps/api/src/auth/guards/jwt-auth.guard.ts (3)
apps/api/src/auth/ability.factory.ts (1)
Injectable(11-63)apps/api/src/auth/strategies/jwt.strategy.ts (1)
Injectable(11-28)apps/api/src/core/decorators/route-access.decorator.ts (2)
RouteAccessType(16-16)ROUTE_ACCESS_METADATA_KEY(22-22)
testing/k6/src/index.ts (1)
packages/schemas/src/auth/auth.ts (3)
$LoginCredentials(11-11)$LoginCredentials(12-15)AuthPayload(7-9)
packages/schemas/src/auth/auth.ts (1)
packages/schemas/src/core/core.ts (1)
Permissions(33-33)
apps/web/src/components/DemoBanner/DemoBanner.tsx (1)
packages/schemas/src/auth/auth.ts (2)
$LoginCredentials(11-11)$LoginCredentials(12-15)
apps/api/src/typings/global.d.ts (2)
packages/schemas/src/auth/auth.ts (1)
TokenPayload(17-25)apps/api/src/auth/auth.types.ts (1)
AppAbility(23-23)
apps/api/src/subjects/subjects.service.ts (1)
apps/api/src/core/prisma.ts (1)
RuntimePrismaClient(67-69)
apps/api/src/auth/__tests__/ability.utils.test.ts (1)
apps/api/src/auth/ability.utils.ts (1)
accessibleQuery(26-35)
apps/api/src/auth/ability.utils.ts (2)
apps/api/src/core/prisma.ts (1)
PrismaModelWhereInputMap(71-77)apps/api/src/auth/auth.types.ts (4)
Permission(23-23)AppAbility(23-23)AppAbilities(23-23)AppAction(23-23)
apps/api/src/users/users.service.ts (1)
apps/api/src/core/types.ts (1)
EntityOperationOptions(3-5)
apps/api/src/instrument-records/instrument-records.controller.ts (2)
packages/schemas/src/instrument-records/instrument-records.ts (6)
$CreateInstrumentRecordData(9-9)$CreateInstrumentRecordData(10-18)$UploadInstrumentRecordsData(25-25)$UploadInstrumentRecordsData(26-36)$UpdateInstrumentRecordData(20-20)$UpdateInstrumentRecordData(21-23)apps/api/src/core/decorators/route-access.decorator.ts (1)
RouteAccess(18-20)
apps/api/src/demo/demo.service.ts (2)
apps/api/src/core/prisma.ts (1)
RuntimePrismaClient(67-69)apps/api/src/core/schemas/mongo-stats.schema.ts (2)
$MongoStats(3-3)$MongoStats(4-8)
apps/api/src/auth/guards/__tests__/jwt-auth.guard.spec.ts (3)
apps/api/src/auth/ability.factory.ts (1)
constructor(13-13)apps/api/src/auth/guards/jwt-auth.guard.ts (1)
constructor(14-19)apps/api/src/core/decorators/route-access.decorator.ts (1)
RouteAccessType(16-16)
apps/api/src/setup/setup.service.ts (2)
apps/api/src/core/prisma.ts (2)
Injectable(8-65)RuntimePrismaClient(67-69)apps/api/src/demo/demo.service.ts (1)
Injectable(35-173)
apps/api/src/sessions/sessions.service.ts (1)
apps/api/src/core/prisma.ts (2)
Injectable(8-65)RuntimePrismaClient(67-69)
🪛 Biome (2.1.2)
apps/api/src/auth/guards/__tests__/jwt-auth.guard.spec.ts
[error] 17-17: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (63)
testing/e2e/src/helpers/types.ts (1)
13-14: LGTM!The
BrowserTargettype is well-defined and appropriate for e2e testing.apps/api/src/core/types.ts (1)
1-1: LGTM - Import path updated to local auth types.Consistent with the migration to local authentication/authorization modules.
apps/api/src/typings/global.d.ts (1)
1-15: LGTM - Express.User augmentation correctly structured.The augmentation properly extends
TokenPayloadand adds theabilityproperty for the new CASL-based authorization system.apps/api/src/core/schemas/env.schema.ts (3)
2-2: LGTM - MongoDB environment integration added.Correctly imports and integrates
$MongoEnvfor database configuration.
5-17: LGTM - Schema composition and type export.The schema properly extends
$BaseEnvwith MongoDB shape and gateway-specific fields. Type export enables proper inference.
18-35: LGTM - Environment validation logic is correct.Transform validates required fields per environment and sets the final
API_PORTappropriately.apps/api/libnest.config.ts (3)
1-3: LGTM - ESLint pragmas added for type augmentation.Appropriate for the namespace augmentation pattern.
12-13: LGTM - Runtime types imported.Correctly imports the runtime Prisma client and environment schema types.
15-20: Namespace augmentation pattern appears syntactically correct, but verify with internal libnest v8 documentation.The module augmentation for
UserTypesinapps/api/libnest.config.tsuses valid TypeScript syntax and is properly isolated to the config file. However, since libnest v8 documentation is not publicly available, confirm this pattern against your internal package documentation or contact the package maintainers to ensure it aligns with the intended v8 API.apps/api/src/setup/setup.controller.ts (1)
5-5: LGTM - RouteAccess import migrated to local decorator.Consistent with the refactor to use local authentication/authorization modules.
package.json (1)
81-85: LGTM - mongodb-memory-server build configuration added.Correctly added to
onlyBuiltDependenciesto ensure native bindings are built properly.apps/api/src/gateway/gateway.controller.ts (1)
5-5: LGTM - RouteAccess import migrated to local decorator.Consistent with the refactor across all controllers.
apps/api/src/sessions/sessions.controller.ts (1)
5-5: LGTM - RouteAccess import migrated to local decorator.Consistent with the refactor across all controllers.
apps/api/src/users/users.controller.ts (1)
1-7: Import refactoring looks good.The migration from libnest to local auth types and decorators is clean. Controller logic remains unchanged.
apps/api/src/auth/__tests__/ability.utils.test.ts (1)
1-24: Test coverage is appropriate.The test cases correctly validate both branches of
accessibleQuery(undefined and defined ability). Mock setup is sound.apps/api/src/auth/auth.controller.ts (1)
1-20: Login endpoint implementation is correct.Public access, proper schema validation, and appropriate HTTP status code for the login operation.
packages/schemas/src/instrument-records/instrument-records.ts (1)
20-23: New update schema added.The union of record and array for
datafield provides flexibility. Note thatz.any()bypasses type safety—this appears intentional for dynamic instrument data but reduces compile-time guarantees.apps/api/src/core/schemas/mongo-stats.schema.ts (1)
1-8: MongoStats schema is well-defined.Simple and appropriate typing for MongoDB statistics.
packages/schemas/src/auth/auth.ts (1)
11-18: Auth schema updates are appropriate.The type-first export pattern is consistent. Adding optional
additionalPermissionsmaintains backward compatibility while enabling permission augmentation.apps/api/src/instruments/instruments.service.ts (1)
1-28: Import migration completed correctly.Moving
accessibleQueryto local utilities aligns with the architecture changes. Service logic remains unchanged.apps/api/src/instruments/instruments.controller.ts (1)
1-9: Import refactoring is consistent.The migration to local auth types and decorators matches the pattern across other controllers.
apps/api/src/assignments/assignments.controller.ts (1)
1-7: LGTM - Clean import refactoring.The import sources have been correctly updated to use local implementations of
RouteAccessandAppAbilitywhile maintainingCurrentUserfrom libnest. The changes align with the PR's goal of moving auth/authorization logic to local modules.apps/api/src/summary/summary.controller.ts (1)
1-6: LGTM - Consistent import refactoring.Import updates match the pattern seen across controllers, moving to local auth types while preserving the multi-entity authorization logic in the
RouteAccessdecorator.apps/web/src/components/LoginForm/LoginForm.tsx (1)
5-9: LGTM - Type rename aligned with schema conventions.The update from
LoginCredentialsto$LoginCredentialsfollows the Zod v4 schema naming pattern. The prop type is correctly updated with no behavioral changes.apps/api/src/subjects/subjects.service.ts (1)
1-7: LGTM - Prisma client type migration.The service correctly migrates from
ExtendedPrismaClienttoRuntimePrismaClientand movesaccessibleQueryto the local auth utilities. The type is properly defined and all usages remain consistent.apps/api/src/demo/demo.service.ts (2)
38-38: LGTM - Prisma client injection updated.The constructor correctly injects
RuntimePrismaClientvia@InjectPrismaClient(), aligning with the new Prisma integration pattern.
158-172: LGTM - DB name derivation with proper error handling.The new private methods correctly use
$runCommandRawto fetch MongoDB stats and parse them with the$MongoStatsschema. Error handling appropriately throwsInternalServerErrorExceptionif parsing fails.apps/web/src/routes/auth/login.tsx (3)
4-4: LGTM - Type import updated.Correctly updated to use
$LoginCredentialsaligned with the schema changes.
15-15: LGTM - Function parameter type updated.The
loginRequestfunction parameter correctly uses the renamed$LoginCredentialstype.
34-34: LGTM - Handler parameter type updated.The
handleLogincallback parameter correctly uses the renamed$LoginCredentialstype.testing/k6/src/index.ts (2)
2-2: LGTM - Test import updated.The k6 load test correctly imports the renamed
$LoginCredentialstype.
54-54: LGTM - Generic type parameter updated.The
client.postcall correctly uses$LoginCredentialsas the generic parameter for the request payload type.apps/api/src/main.ts (1)
1-6: Code changes are correct and properly configured.Both
PrismaModuleOptionsFactory(inapps/api/src/core/prisma.ts) andAuthModule(inapps/api/src/auth/auth.module.ts) are properly exported and correctly integrated. ThePrismaModule.forRootAsyncsetup with the factory pattern is valid, and all imports reference the correct paths.apps/api/src/sessions/sessions.service.ts (1)
1-3: LGTM!The import reorganization and constructor updates are clean. The switch to
RuntimePrismaClientand localaccessibleQueryimport align with the PR's auth system refactoring.Also applies to: 9-10, 18-18, 22-22
apps/api/src/setup/setup.service.ts (2)
1-2: LGTM!Import reorganization and constructor update to use
RuntimePrismaClientwith@InjectPrismaClient()decorator align with the PR's Prisma refactoring.Also applies to: 4-9, 12-12, 19-19, 23-23
83-90: MongoDB command validation confirmed.The implementation correctly validates the MongoDB dropDatabase response. According to MongoDB documentation, the dropDatabase command returns a result with an "ok" field that is 1 on success, which matches the check at line 85. The error handling is appropriate.
apps/api/src/auth/auth.service.ts (4)
1-18: LGTM!Constructor dependencies are appropriate for JWT-based authentication with CASL authorization.
20-31: LGTM!Converting
NotFoundExceptiontoUnauthorizedExceptionprevents user enumeration attacks. Good security practice.
32-35: LGTM!Proper password verification with strict equality check.
37-58: LGTM!Token generation flow is correct: builds payload, computes permissions via
AbilityFactory, and signs JWT with 1-hour expiry. The short expiry is mitigated byignoreExpirationin development mode (seejwt.strategy.tsline 18).apps/api/src/users/users.service.ts (2)
1-1: LGTM!Import reorganization moves
accessibleQueryto local auth utilities while retaining other libnest imports.Also applies to: 5-5
127-142: LGTM!The
includeHashedPasswordoption is correctly implemented. Whentrue, the hashed password is included; whenfalseorundefined, it's omitted. This supports the auth service's need to verify passwords during login.apps/api/src/auth/strategies/jwt.strategy.ts (2)
11-22: LGTM!JWT strategy configuration is correct. Ignoring expiration in development mode is a reasonable development convenience. Using
getOrThrowforSECRET_KEYensures fail-fast behavior.
24-27: LGTM!The
validatemethod correctly reconstructs theAppAbilityfrom JWT permissions and enriches the payload. This integrates properly with the login flow inauth.service.tswhere permissions are computed and embedded in the token.apps/api/package.json (1)
18-22: LGTM! Dependencies properly updated for JWT/CASL authentication.The dependency additions align well with the authentication refactor:
- CASL packages for authorization
- NestJS auth packages for JWT strategy
- Passport packages for authentication middleware
- Libnest v8 upgrade supports the new architecture
Also applies to: 29-30, 47-48, 57-59
testing/e2e/src/global/global.setup.spec.ts (1)
1-1: LGTM! Test updates align with new authentication system.The changes improve test clarity and coverage:
- Updated type import to
$LoginCredentialsfrom schemas- Improved test descriptions
- Added security-focused tests (redirect to login, block duplicate setup)
Also applies to: 8-31, 34-45
apps/api/src/auth/ability.factory.ts (1)
56-62: LGTM! Clean delegation to utility function.apps/api/src/auth/ability.utils.ts (3)
13-18: LGTM! Subject detection properly handles Prisma models.
20-24: LGTM! Ability creation properly configured.
26-35: No security issue found—ability is always passed from authenticated routes.Verification confirms all
accessibleQuerycalls in protected routes receiveabilityfrom the@CurrentUser('ability')decorator. Every controller method is decorated with@RouteAccess, ensuring authentication enforcement, and explicitly passes the injected ability to service methods. The undefined case returns{}as intentional defensive design for flexibility (tested behavior), not a vulnerability in protected contexts.apps/api/src/auth/guards/__tests__/jwt-auth.guard.spec.ts (2)
16-20: LGTM! Mock setup is correct.Note: The Biome warning about shadowing "constructor" is a false positive in this hoisted mock context.
24-131: LGTM! Comprehensive test coverage.The test suite thoroughly covers:
- Public route access
- Missing route metadata error handling
- Authentication failures
- Missing ability error handling
- Permission validation (single and multiple permissions)
apps/api/src/core/decorators/route-access.decorator.ts (1)
1-22: LGTM! Clean decorator implementation with well-defined types.The RouteAccess decorator properly integrates with NestJS metadata system for authorization.
apps/api/src/auth/auth.types.ts (1)
1-23: LGTM! Type definitions properly integrate CASL with Prisma.The types leverage Prisma.TypeMap for type-safe authorization across all models.
apps/api/src/auth/guards/jwt-auth.guard.ts (1)
12-56: LGTM! Robust guard implementation with proper security checks.The guard correctly:
- Validates route metadata exists
- Handles public routes without authentication
- Ensures ability is present and valid before permission checks
- Supports both single and multiple permission requirements
apps/api/src/instrument-records/instrument-records.controller.ts (4)
3-15: LGTM!Import migration from DTOs to zod schema types is clean and consistent with the PR objective.
27-27: LGTM!Schema type correctly replaces DTO.
34-34: LGTM!Schema type correctly replaces DTO.
90-90: LGTM!Schema type correctly replaces DTO with proper destructuring.
apps/api/src/core/prisma.ts (4)
16-33: LGTM!Environment-aware connection logic is clean, and omitting hashedPassword from queries is a good security practice.
35-39: LGTM!Shutdown logic is correct, assuming the assignment bug in
createMemoryConnection()is fixed.
48-64: LGTM!Connection string construction correctly filters empty params and builds environment-specific database name.
67-77: LGTM!Type utilities correctly extract the extended client type and where-input mappings for type-safe Prisma queries.
| switch (payload.basePermissionLevel) { | ||
| case 'ADMIN': | ||
| ability.can('manage', 'all'); | ||
| break; | ||
| case 'GROUP_MANAGER': | ||
| ability.can('manage', 'Assignment', { groupId: { in: groupIds } }); | ||
| ability.can('manage', 'Group', { id: { in: groupIds } }); | ||
| ability.can('read', 'Instrument'); | ||
| ability.can('create', 'InstrumentRecord'); | ||
| ability.can('read', 'InstrumentRecord', { groupId: { in: groupIds } }); | ||
| ability.can('create', 'Session'); | ||
| ability.can('read', 'Session', { groupId: { in: groupIds } }); | ||
| ability.can('create', 'Subject'); | ||
| ability.can('read', 'Subject', { groupIds: { hasSome: groupIds } }); | ||
| ability.can('read', 'User', { groupIds: { hasSome: groupIds } }); | ||
| break; | ||
| case 'STANDARD': | ||
| ability.can('read', 'Group', { id: { in: groupIds } }); | ||
| ability.can('read', 'Instrument'); | ||
| ability.can('create', 'InstrumentRecord'); | ||
| ability.can('read', 'Session', { groupId: { in: groupIds } }); | ||
| ability.can('create', 'Session'); | ||
| ability.can('create', 'Subject'); | ||
| ability.can('read', 'Subject', { groupIds: { hasSome: groupIds } }); | ||
| break; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that create permissions for InstrumentRecord, Session, and Subject are properly scoped at the service layer.
The permission rules allow GROUP_MANAGER and STANDARD users to create records without explicit ability-level constraints on which groups they can create for. Ensure that service methods enforce group membership checks when creating these entities.
🏁 Script executed:
#!/bin/bash
# Verify that create methods in services enforce group scoping
echo "=== Checking InstrumentRecords Service ==="
rg -A 10 -B 2 'create.*InstrumentRecord' --type=ts apps/api/src/instrument-records/
echo -e "\n=== Checking Sessions Service ==="
rg -A 10 -B 2 'create.*Session' --type=ts apps/api/src/sessions/
echo -e "\n=== Checking Subjects Service ==="
rg -A 10 -B 2 'create.*Subject' --type=ts apps/api/src/subjects/Length of output: 8128
🏁 Script executed:
#!/bin/bash
# Find @RouteAccess decorator implementation and Sessions/InstrumentRecords create implementations
echo "=== Finding @RouteAccess decorator ==="
fd -t f '@RouteAccess' --type ts apps/api/src/ || rg -l '@RouteAccess' apps/api/src/ --type ts | head -5
echo -e "\n=== Checking for RouteAccess decorator definition ==="
rg -A 5 -B 2 'RouteAccess.*decorator\|export.*RouteAccess' --type=ts apps/api/src/
echo -e "\n=== Sessions service.create full implementation ==="
rg -A 30 'async create\(' apps/api/src/sessions/sessions.service.ts
echo -e "\n=== InstrumentRecords service.create full implementation ==="
rg -A 30 'async create\(' apps/api/src/instrument-records/instrument-records.service.tsLength of output: 2843
🏁 Script executed:
#!/bin/bash
# Verify groupsService.findById and Sessions controller behavior
echo "=== GroupsService.findById implementation ==="
rg -A 15 'async findById\(' apps/api/src/groups/groups.service.ts
echo -e "\n=== Sessions controller.create full method ==="
rg -B 5 -A 10 'create\(@Body\(\)' apps/api/src/sessions/sessions.controller.ts
echo -e "\n=== Search for RouteAccess definition ==="
rg 'export.*RouteAccess\|function RouteAccess' apps/api/src/ -A 3Length of output: 1017
🏁 Script executed:
#!/bin/bash
# Final verification: check Subjects controller and RouteAccess type
echo "=== Subjects controller.create method ==="
rg -B 3 -A 8 '@RouteAccess.*Subject' apps/api/src/subjects/subjects.controller.ts | grep -A 8 'create\('
echo -e "\n=== RouteAccess in common/auth or shared ==="
find apps/api/src -name '*.ts' -exec grep -l 'export.*RouteAccess\|RouteAccess.*=' {} \; 2>/dev/null | head -3
echo -e "\n=== Check RouteAccess import source ==="
rg 'import.*RouteAccess' apps/api/src/sessions/sessions.controller.tsLength of output: 337
🏁 Script executed:
#!/bin/bash
# Check RouteAccess decorator implementation and Subjects controller
echo "=== RouteAccess decorator implementation ==="
cat apps/api/src/core/decorators/route-access.decorator.ts
echo -e "\n=== Subjects controller.create full method ==="
rg -B 2 -A 12 'create\(@Body' apps/api/src/subjects/subjects.controller.tsLength of output: 1348
🏁 Script executed:
#!/bin/bash
# Check Subjects service create method (not createMany)
echo "=== Subjects service.create implementation ==="
rg -A 20 'async create\(' apps/api/src/subjects/subjects.service.ts | head -30Length of output: 647
Authorization bypass in Sessions and Subjects create endpoints.
Verification confirms the original concern: Session and Subject create operations bypass authorization checks.
- Sessions: Controller doesn't pass
abilityto service; service callsgroupsService.findById(groupId)withoutability, allowing any user to create sessions for any group. - Subjects: Controller doesn't pass
abilityto service; service.create() doesn't useaccessibleQueryat all, allowing any user to create subjects. - InstrumentRecords: Properly scoped—controller passes
abilitythrough the service chain and checks group access.
Both Sessions and Subjects endpoints need ability enforcement at the service layer, following the InstrumentRecords pattern: pass ability from controller and use accessibleQuery in service create methods.
| private async createMemoryConnection(): Promise<string> { | ||
| // prevent mongodb-memory-server from being included in the production bundle | ||
| const { MongoMemoryReplSet } = await import('mongodb-memory-server'); | ||
| const replSet = await MongoMemoryReplSet.create({ replSet: { count: 1, name: 'rs0' } }); | ||
| return new URL(replSet.getUri('test')).href; | ||
| } |
There was a problem hiding this comment.
Memory leak: replSet not stored for cleanup.
The MongoMemoryReplSet instance is created but not assigned to this.memoryReplSet, so onApplicationShutdown() cannot stop it, causing a resource leak.
Apply this diff:
private async createMemoryConnection(): Promise<string> {
// prevent mongodb-memory-server from being included in the production bundle
const { MongoMemoryReplSet } = await import('mongodb-memory-server');
- const replSet = await MongoMemoryReplSet.create({ replSet: { count: 1, name: 'rs0' } });
- return new URL(replSet.getUri('test')).href;
+ this.memoryReplSet = await MongoMemoryReplSet.create({ replSet: { count: 1, name: 'rs0' } });
+ return new URL(this.memoryReplSet.getUri('test')).href;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private async createMemoryConnection(): Promise<string> { | |
| // prevent mongodb-memory-server from being included in the production bundle | |
| const { MongoMemoryReplSet } = await import('mongodb-memory-server'); | |
| const replSet = await MongoMemoryReplSet.create({ replSet: { count: 1, name: 'rs0' } }); | |
| return new URL(replSet.getUri('test')).href; | |
| } | |
| private async createMemoryConnection(): Promise<string> { | |
| // prevent mongodb-memory-server from being included in the production bundle | |
| const { MongoMemoryReplSet } = await import('mongodb-memory-server'); | |
| this.memoryReplSet = await MongoMemoryReplSet.create({ replSet: { count: 1, name: 'rs0' } }); | |
| return new URL(this.memoryReplSet.getUri('test')).href; | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/core/prisma.ts around lines 41 to 46, the MongoMemoryReplSet
created in createMemoryConnection() is not assigned to this.memoryReplSet so
onApplicationShutdown() cannot stop it; assign the created replSet to
this.memoryReplSet (this.memoryReplSet = replSet) before returning the URI so
the instance can be cleaned up, and ensure the class has a matching
memoryReplSet property declaration if not already present.
apps/api/src/instrument-records/instrument-records.controller.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
testing/e2e/playwright.config.ts (1)
75-76: Good addition for CI visibility.Piping stderr unconditionally and stdout only in CI strikes the right balance between local dev silence and CI diagnostics.
If you'd like to reduce the duplication, consider extracting the pattern:
+const webServerStreams = { + stderr: 'pipe' as const, + stdout: (process.env.CI ? 'pipe' : 'ignore') as const +}; + export default defineConfig({Then reference it in each webServer:
{ command: 'pnpm dev:test', cwd: path.resolve(import.meta.dirname, '../../apps/api'), gracefulShutdown: { signal: 'SIGINT', timeout: 1000 }, - stderr: 'pipe', - stdout: process.env.CI ? 'pipe' : 'ignore', + ...webServerStreams, timeout: 10_000, url: `http://localhost:${apiPort}` },Also applies to: 87-88, 99-100
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/runtime-core/src/define.ts(2 hunks)packages/runtime-core/src/types/instrument.series.ts(1 hunks)testing/e2e/playwright.config.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/runtime-core/src/define.ts (3)
packages/runtime-core/src/types/instrument.base.ts (1)
InstrumentLanguage(223-223)vendor/[email protected]/src/v3.d.ts (1)
Omit(820-820)packages/runtime-core/src/types/instrument.series.ts (1)
SeriesInstrument(22-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint-and-test
🔇 Additional comments (3)
packages/runtime-core/src/types/instrument.series.ts (1)
6-6: LGTM! API promotion is clear.Promoting SeriesInstrument from @beta to @public aligns with the new defineSeriesInstrument helper in define.ts.
packages/runtime-core/src/define.ts (2)
8-8: LGTM!Import supports the new defineSeriesInstrument helper.
65-72: LGTM! Pattern matches defineInstrument.The function correctly applies the same
__runtimeVersion: 1pattern used by defineInstrument. TypeScript enforceskind: 'SERIES'via the SeriesInstrument type constraint.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/playground/src/components/Header/LoginForm/LoginForm.tsx (1)
1-1: Consider UX-driven ordering or inline disable.The eslint disable suggests the object properties don't follow project sorting conventions. If the current order (username before password) is intentional for UX, consider adding a comment explaining this. Otherwise, sort properties alphabetically and remove the disable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/api/libnest.config.ts(1 hunks)apps/api/src/auth/auth.controller.ts(1 hunks)apps/api/src/auth/auth.service.ts(1 hunks)apps/api/src/typings/global.d.ts(1 hunks)apps/playground/src/components/Header/LoginForm/LoginForm.tsx(1 hunks)apps/playground/src/components/Header/LoginForm/index.ts(1 hunks)apps/playground/src/store/index.ts(2 hunks)apps/playground/src/store/slices/auth.slice.ts(1 hunks)apps/playground/src/store/types.ts(2 hunks)packages/schemas/src/auth/auth.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/api/libnest.config.ts
- apps/api/src/typings/global.d.ts
- apps/api/src/auth/auth.service.ts
🧰 Additional context used
🧬 Code graph analysis (5)
apps/playground/src/components/Header/LoginForm/LoginForm.tsx (1)
packages/schemas/src/auth/auth.ts (2)
$LoginCredentials(11-11)$LoginCredentials(12-15)
apps/playground/src/store/slices/auth.slice.ts (1)
apps/playground/src/store/types.ts (2)
SliceCreator(87-92)AuthSlice(37-40)
apps/api/src/auth/auth.controller.ts (3)
apps/api/src/core/decorators/route-access.decorator.ts (1)
RouteAccess(18-20)apps/api/libnest.config.ts (1)
RequestUser(21-23)packages/schemas/src/auth/auth.ts (2)
$LoginCredentials(11-11)$LoginCredentials(12-15)
apps/playground/src/store/index.ts (1)
apps/playground/src/store/slices/auth.slice.ts (1)
createAuthSlice(3-10)
packages/schemas/src/auth/auth.ts (1)
packages/schemas/src/core/core.ts (1)
Permissions(33-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint-and-test
🔇 Additional comments (9)
packages/schemas/src/auth/auth.ts (2)
11-15: LGTM! Non-empty string validation added.The
.min(1)validation on username and password fields prevents empty string submissions, improving input validation.
18-18: LGTM! Additional permissions field added.The optional
additionalPermissionsfield extends the token payload capabilities without breaking changes.apps/api/src/auth/auth.controller.ts (1)
15-20: LGTM! Protected endpoint with proper authorization.The endpoint correctly delegates to the service layer and enforces permission checks via
RouteAccess.apps/playground/src/components/Header/LoginForm/LoginForm.tsx (1)
10-30: LGTM! Simple and effective form implementation.The component correctly uses the
$LoginCredentialsschema for validation and delegates submission handling to the parent via theonSubmitprop.apps/playground/src/store/index.ts (2)
9-9: LGTM: Auth slice import follows existing pattern.The import is correctly placed alongside other slice imports.
23-23: Add token lifecycle management; currently missing.Verification confirms token handling is incomplete. The auth slice only stores and sets the token—there's no expiration tracking, refresh mechanism, or validation. Tokens are used directly in API calls without safeguards. Implement:
- Token expiration detection and refresh flow
- API interceptor to inject/validate tokens and handle 401 responses
- Optional: Route guards to prevent navigation with expired tokens
apps/playground/src/store/slices/auth.slice.ts (1)
1-10: LGTM: Auth slice implementation is correct.The slice follows Zustand + immer patterns correctly. The minimal implementation (just token storage) is appropriate—business logic like validation and expiration should be handled by auth services or middleware.
apps/playground/src/store/types.ts (2)
37-40: LGTM: AuthSlice type is well-defined.The type correctly allows
null | stringforaccessToken, enabling token clearing on logout.
85-85: LGTM: AppStore properly extended with AuthSlice.The type composition is correct and maintains type safety across all store slices.
| @ApiOperation({ summary: 'Login' }) | ||
| @HttpCode(HttpStatus.OK) | ||
| @Post('login') | ||
| @RouteAccess('public') | ||
| async login(@Body() credentials: $LoginCredentials): Promise<{ accessToken: string }> { | ||
| return this.authService.login(credentials); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Consider rate limiting and enhance API documentation.
The login endpoint is publicly accessible and lacks visible rate limiting, making it vulnerable to brute-force attacks. Also consider documenting error responses (401, 400) using @ApiResponse decorators.
Verify rate limiting configuration:
If rate limiting isn't configured, consider adding it:
import { Throttle } from '@nestjs/throttler';
// Add above the login method:
@Throttle({ default: { limit: 5, ttl: 60000 } }) // 5 attempts per minute🏁 Script executed:
#!/bin/bash
# Check if rate limiting is configured globally or on auth routes
rg -nP --type=ts -C5 'ThrottlerModule|RateLimit|@Throttle'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check app.module.ts for ThrottlerModule import
fd -t f "app.module.ts" --exec cat {}
# Check main.ts for global guards or rate limiting setup
fd -t f "main.ts" --exec cat {}
# Search for any throttler-related imports in the codebase
rg -n "throttler|rate.limit" -i --type=tsLength of output: 6568
Rate limiting is not configured. Add throttling to the login endpoint to prevent brute-force attacks.
Verification confirms no rate limiting is configured globally or on the auth routes. The login endpoint remains unprotected.
Add the @Throttle() decorator from @nestjs/throttler above the login method:
@Throttle({ default: { limit: 5, ttl: 60000 } })Also document error responses using @ApiResponse decorators for 401 and 400 status codes.
🤖 Prompt for AI Agents
In apps/api/src/auth/auth.controller.ts around lines 22 to 28, the login
endpoint lacks rate limiting and response documentation; add the Throttle
decorator from @nestjs/throttler directly above the login method with the
provided settings (limit 5, ttl 60000) and add @ApiResponse decorators for 401
(Unauthorized) and 400 (Bad Request) to document error responses; also update
imports to include Throttle from @nestjs/throttler and ApiResponse from
@nestjs/swagger, placing the decorators immediately above the method signature
so Nest registers throttling and Swagger documents the error responses.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Summary by CodeRabbit
New Features
Changes